Skip to content

fix(gcs): paginate object prefixes with maxResults - #189

Open
electrum wants to merge 1 commit into
floci-io:mainfrom
electrum:user/electrum/gcs-list-max-results
Open

fix(gcs): paginate object prefixes with maxResults#189
electrum wants to merge 1 commit into
floci-io:mainfrom
electrum:user/electrum/gcs-list-max-results

Conversation

@electrum

@electrum electrum commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Applies maxResults to the combined object and prefix listing, with one page token covering both result types.

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

GCP Compatibility

The behavior was reproduced in real GCS with objects under a/ and b/:

$ gcloud storage cp /dev/null gs://$BUCKET/a/file
Copying file:///dev/null to gs://$BUCKET/a/file
$ gcloud storage cp /dev/null gs://$BUCKET/b/file
Copying file:///dev/null to gs://$BUCKET/b/file
$ curl --include --get \
    --header "Authorization: Bearer $(gcloud auth print-access-token)" \
    --data-urlencode 'delimiter=/' \
    --data-urlencode 'maxResults=1' \
    "https://storage.googleapis.com/storage/v1/b/$BUCKET/o"
HTTP/2 200
content-type: application/json; charset=UTF-8

{
  "kind": "storage#objects",
  "nextPageToken": "CgJhLw==",
  "prefixes": [
    "a/"
  ]
}

Floci previously returned every prefix and no page token because only items[] participated in pagination. Real GCS was also checked with includeTrailingDelimiter=true: a same-named prefix and object are both returned when maxResults=1, and together consume one result slot. With versioning enabled, each generation remains a separate, generation-ordered result slot while the same-named prefix is returned once, keeping continuation pages stable.

Checklist

  • ./mvnw test passes locally
  • New or updated integration test added
  • Commit messages follow Conventional Commits

@greptile-apps

greptile-apps Bot commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Greptile Summary

This PR fixes GCS delimiter-based listings so maxResults and page tokens operate over the combined sequence of objects and prefixes.

  • Groups same-name object generations while pairing a trailing-delimiter prefix with only the first generation.
  • Sorts generations deterministically to keep offset-based continuation pages stable.
  • Adds REST integration coverage for combined pagination and versioned trailing-delimiter objects.
  • Adds Java SDK compatibility coverage for paginated directory prefixes.

Confidence Score: 5/5

The PR appears safe to merge, with the previously reported generation-loss and unstable-pagination findings fully addressed.

Same-name generations are preserved as separate results and sorted deterministically before offset pagination, while prefixes and objects share the intended page budget. Both previous review threads are resolved, and no new actionable defect remains.

Important Files Changed
Filename Overview
src/main/java/io/floci/gcp/services/gcs/GcsObjectController.java Combines objects and prefixes into one deterministic result sequence before applying pagination.
src/test/java/io/floci/gcp/services/gcs/GcsListFilterRestIntegrationTest.java Covers combined object-prefix pages, shared result slots, and stable version-generation continuation pages.
compatibility-tests/sdk-test-java/src/test/java/io/floci/gcp/test/GcsListFiltersTest.java Verifies through the Java GCS SDK that directory prefixes honor page size and expose a continuation token.

Flowchart

 
%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[List matching objects] --> B{Delimiter supplied?}
    B -- No --> C[Convert each object to a listed result]
    B -- Yes --> D[Group objects by name]
    D --> E[Sort same-name generations numerically]
    E --> F[Merge ordered object names and prefixes]
    F --> G[Pair a same-name prefix with first object generation]
    G --> H[Emit remaining generations separately]
    C --> I[Paginate combined listed results]
    H --> I
    I --> J[Split current page into items and prefixes]
    J --> K[Return one shared nextPageToken]
Loading

Reviews (3): Last reviewed commit: "fix(gcs): paginate object prefixes with ..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot reviewed Sep 9, 2026
View reviewed changes
Comment thread src/main/java/io/floci/gcp/services/gcs/GcsObjectController.java Outdated
@electrum
electrum force-pushed the user/electrum/gcs-list-max-results branch from 52099c9 to 27131d5 Compare September 9, 2026 05:06
greptile-apps[bot]
greptile-apps Bot reviewed Sep 9, 2026
View reviewed changes
Comment thread src/main/java/io/floci/gcp/services/gcs/GcsObjectController.java
@electrum
electrum force-pushed the user/electrum/gcs-list-max-results branch from 27131d5 to 17e0911 Compare September 9, 2026 05:18
@hectorvent hectorvent added bug Something isn't working storage Cloud Storage (GCS) labels Sep 10, 2026
@hectorvent hectorvent mentioned this pull request Sep 12, 2026
Merged
7 tasks
@hectorvent

hectorvent commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Thank you, and the curl showing nextPageToken next to a lone prefix is the clearest possible way to make this case.

It is confirmed by the spec as well as the probe: objects.list documents maxResults as the "Maximum number of items plus prefixes to return in a single page of responses. As duplicate prefixes are omitted, fewer total results may be returned than requested" (objects.list). Floci was paginating items only and re-sending every prefix on each page, so this is a real gap rather than a refinement.

The two things the doc does not cover, the shared slot for a same-named prefix and object, and each generation taking its own slot, are the ones your capture settles, and both are pinned by the new tests.

No blockers from my side. #188 touches the same lines, so whichever lands second will want a small rebase.

hectorvent
hectorvent approved these changes Sep 12, 2026
View reviewed changes

@hectorvent hectorvent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contract claims confirmed verbatim against the storage v1 reference. Approving the code; needs a rebase now that #188 has landed.

@hectorvent hectorvent added waiting-contributor Deprecated: use waiting-author waiting-author Review posted; waiting on the PR author to respond and removed waiting-contributor Deprecated: use waiting-author labels Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments
@hectorvent hectorvent hectorvent approved these changes

Assignees

No one assigned

Labels

bug Something isn't working storage Cloud Storage (GCS) waiting-author Review posted; waiting on the PR author to respond

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@electrum @hectorvent